Revert "[fpmsyncd]: add libnexthopgroup in Makefile.am to support rib/fib" - #4657
Conversation
…/fib (sonic-net#4394)" This reverts commit e6c482c. Signed-off-by: Lawrence Lee <lawlee@microsoft.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
This PR reverts the earlier addition of a libnexthopgroup build/deployment dependency in SWSS, primarily to unblock CI/builds now that libnexthopgroup is no longer reliably produced in the common-libs pipeline during the libyang1→libyang3 migration.
Changes:
- Remove
-lnexthopgroupfromfpmsyncdlink flags. - Stop downloading/including
libnexthopgroupandlibnexthopgroup-devartifacts in Azure Pipeline build templates. - Remove
libnexthopgrouphandling from the docker-sonic-vs image build/purge/install steps.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
fpmsyncd/Makefile.am |
Drops the libnexthopgroup link dependency from fpmsyncd. |
.azure-pipelines/docker-sonic-vs/Dockerfile |
Removes libnexthopgroup from purge/install paths for the VS docker image. |
.azure-pipelines/build-template.yml |
Stops downloading libnexthopgroup deb artifacts from common libs. |
.azure-pipelines/build-docker-sonic-vs-template.yml |
Removes the pipeline steps that fetched/copied libnexthopgroup into the VS docker build context. |
theasianpianist
changed the title
theasianpianist
changed the title
darius-nexthop
left a comment
darius-nexthop
left a comment
There was a problem hiding this comment.
Thanks, CI is failing for my PR as well.
|
@eddieruan-alibaba for viz |
|
@prsunny This revert doesn't make sense, right? We are on the way to upstream RIB/FIB codes. Thsi revert would break those PRs. |
This change was breaking CI as it added a dependency on There's no reason to update the build dependency for SWSS before it's actually in use. This change can be included in the main RIB/FIB SWSS PR. Please check my comment here: #4395 (comment) |
#4394 PR was raised in Mar 26 and merged 3 weeks ago with all sanity pass. If CI was fail, how to make this CI fail expose to PR phase ? @prsunny Also, why not contact the owner to fix it instead of immediate revert? What is the revert protocol here? @prsunny It is NOT a good practice to revert PR which has been there for some time. We should respect other folks contributions, especially considering the community PR takes a long time to get reviewed. We need to discuss this issue. We need to enable the compile due to this lib would be used in the following PRs. |
|
ack @eddieruan-alibaba , this is a temporary WA to unblock other PRs. As I understand corresponding sonic-buildimage changes caused this regression. Could you check the notes on description? Once that's built, we can revert this PR. @theasianpianist , could you share any other details on the issue and how we can get this back? |
|
I do see this note from @theasianpianist - @eddieruan-alibaba , can you check?
|
I saw his comments was made yesterday almost the same time as reverting happening. My concern is on this revert event.
|
|
Then why does this PR get immediately revert, not follow the protocol? Any reason behind? |
|
Cherry-pick PR to 202605: #4668 |
…27880) Add LIBFIB to the lib-packages build target so that the common lib pipeline includes libnexthopgroup in its artifacts. <!-- Please make sure you've read and understood our contributing guidelines: https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md ** Make sure all your commits include a signature generated with `git commit -s` ** If this is a bug fix, make sure your description includes "fixes #xxxx", or "closes #xxxx" or "resolves #xxxx" Please provide the following information: --> #### Why I did it Libnexthopgroup will be used in the compilation of swss and sonic frr, and libnexthopgroup will be explicitly added to common lib for compilation of other repo ‘s pipeline. related swss PR: sonic-net/sonic-swss#4395 related swss PR: sonic-net/sonic-swss#4657 ##### Work item tracking - Microsoft ADO **(number only)**: #### How I did it #### How to verify it <!-- If PR needs to be backported, then the PR must be tested against the base branch and the earliest backport release branch and provide tested image version on these two branches. For example, if the PR is requested for master, 202211 and 202012, then the requester needs to provide test results on master and 202012. --> #### Which release branch to backport (provide reason below if selected) <!-- - Note we only backport fixes to a release branch, *not* features! - Please also provide a reason for the backporting below. - e.g. - [x] 202006 --> - [ ] 202305 - [ ] 202311 - [ ] 202405 - [ ] 202411 - [ ] 202505 - [ ] 202511 - [ ] 202605 #### Tested branch (Please provide the tested image version) <!-- - Please provide tested image version - e.g. - [x] 20201231.100 --> - [ ] <!-- image version 1 --> - [ ] <!-- image version 2 --> #### Description for the changelog <!-- Write a short (one line) summary that describes the changes in this pull request for inclusion in the changelog: --> <!-- Ensure to add label/tag for the feature raised. example - PR#2174 under sonic-utilities repo. where, Generic Config and Update feature has been labelled as GCU. --> #### Link to config_db schema for YANG module changes <!-- Provide a link to config_db schema for the table for which YANG model is defined Link should point to correct section on https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md --> Signed-off-by: Sonic Build Admin <sonicbld@microsoft.com> #### A picture of a cute animal (not mandatory but encouraged)
…onic-net#27880) Add LIBFIB to the lib-packages build target so that the common lib pipeline includes libnexthopgroup in its artifacts. <!-- Please make sure you've read and understood our contributing guidelines: https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md ** Make sure all your commits include a signature generated with `git commit -s` ** If this is a bug fix, make sure your description includes "fixes #xxxx", or "closes #xxxx" or "resolves #xxxx" Please provide the following information: --> #### Why I did it Libnexthopgroup will be used in the compilation of swss and sonic frr, and libnexthopgroup will be explicitly added to common lib for compilation of other repo ‘s pipeline. related swss PR: sonic-net/sonic-swss#4395 related swss PR: sonic-net/sonic-swss#4657 ##### Work item tracking - Microsoft ADO **(number only)**: #### How I did it #### How to verify it <!-- If PR needs to be backported, then the PR must be tested against the base branch and the earliest backport release branch and provide tested image version on these two branches. For example, if the PR is requested for master, 202211 and 202012, then the requester needs to provide test results on master and 202012. --> #### Which release branch to backport (provide reason below if selected) <!-- - Note we only backport fixes to a release branch, *not* features! - Please also provide a reason for the backporting below. - e.g. - [x] 202006 --> - [ ] 202305 - [ ] 202311 - [ ] 202405 - [ ] 202411 - [ ] 202505 - [ ] 202511 - [ ] 202605 #### Tested branch (Please provide the tested image version) <!-- - Please provide tested image version - e.g. - [x] 20201231.100 --> - [ ] <!-- image version 1 --> - [ ] <!-- image version 2 --> #### Description for the changelog <!-- Write a short (one line) summary that describes the changes in this pull request for inclusion in the changelog: --> <!-- Ensure to add label/tag for the feature raised. example - PR#2174 under sonic-utilities repo. where, Generic Config and Update feature has been labelled as GCU. --> #### Link to config_db schema for YANG module changes <!-- Provide a link to config_db schema for the table for which YANG model is defined Link should point to correct section on https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md --> Signed-off-by: Sonic Build Admin <sonicbld@microsoft.com> #### A picture of a cute animal (not mandatory but encouraged)
…/fib (sonic-net#4394)" (sonic-net#4657) This reverts sonic-net#4394. What I did Remove build dependency on libnexthopgroup to unblock CI failures. This lib is not actively used in SWSS at the moment. Why I did it sonic-net/sonic-buildimage#27629 is part of the libyang1 to libyang3 migration and causes libnexthopgroup to not be built as part of the common-libs pipeline. Previously when the migration was in progress, both libyang1 and 3 were being built. All packages which were dependent on libyang3 (including libnexthopgroup transitively via FRR) were built as part of common-libs due to some serialization logic to prevent libyang1 and 3 from conflicting in sonic-buildimage at build time. Now that libyang1 is completely removed, this serialization logic doesn't exist anymore and libnexthopgroup is not built as part of common-libs, which causes the SWSS build to fail since libnexthopgroup is missing. Signed-off-by: Yogapriya Mohankumar <ymohanku@cisco.com>
This reverts #4394.
What I did
Remove build dependency on libnexthopgroup to unblock CI failures. This lib is not actively used in SWSS at the moment.
Why I did it
sonic-net/sonic-buildimage#27629 is part of the libyang1 to libyang3 migration and causes libnexthopgroup to not be built as part of the common-libs pipeline. Previously when the migration was in progress, both libyang1 and 3 were being built. All packages which were dependent on libyang3 (including libnexthopgroup transitively via FRR) were built as part of common-libs due to some serialization logic to prevent libyang1 and 3 from conflicting in sonic-buildimage at build time. Now that libyang1 is completely removed, this serialization logic doesn't exist anymore and libnexthopgroup is not built as part of common-libs, which causes the SWSS build to fail since libnexthopgroup is missing.
How I verified it
Details if related